Add FW loiter size and orientation control #24625
Conversation
Params 3 and 4 handling are missing. This adds it in. Param 3 now will respond to a loiter size and param 4 to 0/1 for CW/CCW. Also, on alt only changes if you omit param3/4 it explicitly does what you have set as your default (rather than last done which is how it is now) to ensure you get the default unless the param is passed.
|
Also, this allows QGC to expose loiter size and direction control |
| @@ -392,7 +410,7 @@ void Navigator::run() | |||
| rep->current.loiter_pattern = position_setpoint_s::LOITER_TYPE_ORBIT; | |||
| } | |||
|
|
|||
| rep->current.loiter_direction_counter_clockwise = curr->current.loiter_direction_counter_clockwise; | |||
| rep->current.loiter_direction_counter_clockwise = (PX4_ISFINITE(cmd.param4) ? (cmd.param4 >= 1.0f) : false); | |||
There was a problem hiding this comment.
Do we really need this separate handling? In the only_alt_change_requested case we, as the name says, want to keep everything beside the altitude as it was set before, no?
There was a problem hiding this comment.
I added this so when a 'change-altitude-only' command is sent (i.e. param3 and param4 omitted), we explicitly apply the vehicle’s default loiter radius and direction, exactly the behavior people expect today. Without this, the vehicle would continue using the previous radius/direction settings, which can be surprising.
Also, this makes the behavior match the MAVLink spec’s intent that omitted parameters revert to defaults.
That said, I don't actually care but I do think it's the right thing.
There was a problem hiding this comment.
@ryanjAA I still challenge that it's expected that if you send a "change altitude" through QGC the loiter direction changes (see screen-recording).
Could QGC send a full GoTo setpoint again? Probably, but it's currently not doing it, thus this change changes the current behavior.
Screencast.from.30-04-25.17.48.25.webm
There was a problem hiding this comment.
Also, this makes the behavior match the MAVLink spec’s intent that omitted parameters revert to defaults.
But then the lat/long fields should also not being assumed but reset to e.g. the current vehicle position. Currently there it keeps the lat/long from the previous setpoint.
Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
|
And alternative is to use https://mavlink.io/en/messages/common.html#MAV_CMD_DO_ORBIT. We already have support on the PX4 side for it, incl. direction change. |
Fixes: #24612
Params 3 and 4 handling are missing. This adds it in.
Param 3 now will respond to a loiter size and param 4 to 0/1 for CW/CCW.
Also, on alt only changes if you omit param3/4 it explicitly does what you have set as your default (rather than last done which is how it is now) to ensure you get the default unless the param is passed.
https://logs.px4.io/plot_app?log=61232f4d-fa71-4965-bd37-0be6c4740bb8
(Main from today)
https://discord.com/channels/1022170275984457759/1022186266957201480/1224558850179469414
https://discord.com/channels/1022170275984457759/1022186266957201480/1354505832426901546